-
Notifications
You must be signed in to change notification settings - Fork 67
jextract (ffm, jni): Subscripts support #459
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| public subscript() -> Int { | ||
| get { return len } | ||
| set { len = newValue } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about a case with parameters?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't notice I lost them in git stash :/ Fixed
|
|
||
| public subscript() -> Int { | ||
| get { return len } | ||
| set { len = newValue } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can it set some other value and not the len perhaps? Just so we don't step on values when we write bigger tests
| try (var arena = AllocatingSwiftArena.ofConfined()) { | ||
| MySwiftStruct s = MySwiftStruct.init(1337, 42, arena); | ||
| long currentValue = s.getSubscript(); | ||
| s.setSubscript(66); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
| import java.lang.invoke.*; | ||
| import java.util.*; | ||
| import java.nio.charset.StandardCharsets; | ||
| """,]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably skip the imports from these tests; the more these tests assert text the more annoying it is to change things in the future;
Let's just test the things that this specific "feature" is
| super(segment, arena); | ||
| } | ||
| """ | ||
| ]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove those thunks too, let's not assert anything that's not related to the actual subscripts -- like these chunks
| $ensureAlive(); | ||
| swiftjava_SwiftModule_MyStruct_subscript$set.call(index, newValue, this.$memorySegment()); | ||
| """, | ||
| ]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's the chunks which we do want to assert on, looking good, thanks!
| return MyStruct.$typeMetadataAddressDowncall(); | ||
| } | ||
| """ | ||
| ]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same request about only the subscript text chunks as in the ffm one
|
Very nice, just some nitpicks about the tests |
Resolves: #352